-
Notifications
You must be signed in to change notification settings - Fork 0
Simple mechs merge into 2025 #118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 2025
Are you sure you want to change the base?
Conversation
For some reason all the files were not in lib199 but only in carlmontrobotics
Removed state enum cause did not able to work nicely how I thought
Created a detailed plug and play common mechs that allow programming not to waste time reinventing the wheel each year
Hopefully it works
timtogan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good! I mostly skimed over the code, so I'll prob do another review when I have time. Also fyi, some comments I left only for one mech but it can be applied to (all) other mechs as well
| * @param armKPID double array with 3 values, kP, kI, kD | ||
| * @param bottomLimit lowest angle in degrees the arm can safely achieve with 0 being horizontal | ||
| * @param topLimit highest angle in degrees the arm can safely achieve with 0 being horizontal | ||
| * @throws IllegalArgumentException if MotorType is not one of the 3 expected {@link MotorConfig}s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 as we added solo vortexes and neo 2.0
| armKP = 0; | ||
| armKI = 0; | ||
| armKD = 0; | ||
| DriverStation.reportWarning("ArmPID is off", true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would print why ArmPID is off, ie ArmPID is off because KP is less than 0
| armKG = 0; | ||
| armKV = 0; | ||
| armKA = 0; | ||
| DriverStation.reportWarning("ArmFeedForward is off", true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
| * @throws IllegalArgumentException if the bottom limit is higher or equal to the top limit | ||
| */ | ||
| public static ArmConfig motorWithPID(int armMasterMotorId, MotorConfig armMasterMotorType, boolean armMasterInversed, double[] armKPID, double bottomLimit, double topLimit) { | ||
| return new ArmConfig(1, 1, 14, null, null, null, armMasterMotorId, armMasterMotorType, armMasterInversed, armKPID, defaultArmPIDTolerance, false, null, null, -1, 0, bottomLimit, topLimit); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return new ArmConfig(1, 1, 14, null, null, null, armMasterMotorId, armMasterMotorType, armMasterInversed, armKPID, defaultArmPIDTolerance, false, null, null, -1, 0, bottomLimit, topLimit); | |
| return new ArmConfig(1, 1, 12, null, null, null, armMasterMotorId, armMasterMotorType, armMasterInversed, armKPID, defaultArmPIDTolerance, false, null, null, -1, 0, bottomLimit, topLimit); |
| } | ||
|
|
||
| /** | ||
| * Checks PID to have 3 values, updates {@link #armPIDExists} and makes sure the values are legitimate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is armPIDExists, I can't find it refrenced anywhere but in here
| /** | ||
| * @deprecated Use {@link #resetFieldOrientation()} instead | ||
| */ | ||
| @Deprecated | ||
| public void resetHeading() { | ||
| gyro.reset(); | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /** | |
| * @deprecated Use {@link #resetFieldOrientation()} instead | |
| */ | |
| @Deprecated | |
| public void resetHeading() { | |
| gyro.reset(); | |
| } |
| /** | ||
| * @deprecated Use {@link #setMode(Mode)} instead | ||
| * Changes between IdleModes | ||
| */ | ||
| @Deprecated | ||
| public void toggleMode() { | ||
| for (SwerveModule module : modules) | ||
| module.toggleMode(); | ||
| } | ||
| /** | ||
| * @deprecated Use {@link #setMode(Mode)} instead | ||
| */ | ||
| @Deprecated | ||
| public void brake() { | ||
| for (SwerveModule module : modules) | ||
| module.brake(); | ||
| } | ||
| /** | ||
| * @deprecated Use {@link #setMode(Mode)} instead | ||
| */ | ||
| @Deprecated | ||
| public void coast() { | ||
| for (SwerveModule module : modules) | ||
| module.coast(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /** | |
| * @deprecated Use {@link #setMode(Mode)} instead | |
| * Changes between IdleModes | |
| */ | |
| @Deprecated | |
| public void toggleMode() { | |
| for (SwerveModule module : modules) | |
| module.toggleMode(); | |
| } | |
| /** | |
| * @deprecated Use {@link #setMode(Mode)} instead | |
| */ | |
| @Deprecated | |
| public void brake() { | |
| for (SwerveModule module : modules) | |
| module.brake(); | |
| } | |
| /** | |
| * @deprecated Use {@link #setMode(Mode)} instead | |
| */ | |
| @Deprecated | |
| public void coast() { | |
| for (SwerveModule module : modules) | |
| module.coast(); | |
| } |
| if (armFollowId != null) { | ||
| for (MotorConfig config: armFollowMotorType) { | ||
| if (config != MotorConfig.NEO && config != MotorConfig.NEO_VORTEX && config != MotorConfig.NEO_550) { | ||
| throw new IllegalArgumentException("What is this config??? If null pls change, if not null you're cooked"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it's a neo 2.0 or a solo vortex :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also I feel like this is not really needed, as I don't nesseceraly see how this is better than a null check and it adds burden, as you need to modify this whenever a motor is added to motorConfig
Please merge this tim, it works